🐛 Fix nil pointer dereferences and panics across GCP provider#6942
🐛 Fix nil pointer dereferences and panics across GCP provider#6942
Conversation
- discovery.go: Fix inverted nil check (buckets == nil -> buckets.Error != nil) - discovery.go: Replace log.Fatal with error returns in ListRepository - cloudrun.go: Break on iterator error instead of falling through to nil deref - cloudrun.go: Guard nil timestamps, Timeout durations, and SecretKeyRef - sql.go: Guard nil Settings and BackupRetentionSettings - kms.go: Add missing error checks after CreateResource/cryptoKeyVersionToMql - kms.go: Guard nil CertChains and nil timestamps - pubsub.go: Guard nil policy.InternalProto before accessing Bindings - common.go: Add bounds checks on URL parsing (getNetworkByUrl, getSubnetworkByUrl, getDiskIdByUrl) - common.go: Guard empty PlatformIds in getAssetIdentifier - common.go: Add durationSecondsToTimePtr helper for nil-safe duration conversion - gke.go: Add bounds checks on network/subnet path splitting - redis.go: Guard nil CreateTime on instances and clusters - filestore.go: Guard nil CreateTime on instances - dataproc.go: Guard nil nested structs in VirtualClusterConfig and nodePoolTargetToMql Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![mondoo-code-review[bot]](https://avatars.githubusercontent.com/in/2834902?s=60&v=4){kind=small}
| } | ||
| buckets := storage.Data.GetBuckets() | ||
| if buckets == nil { | ||
| if buckets.Error != nil { |
There was a problem hiding this comment.
🟡 warning — The original code checked if buckets == nil to guard against a nil buckets value. The new code if buckets.Error != nil will panic if buckets itself is nil (i.e., if GetBuckets() returns a nil plugin result). The original nil check was likely intentional — consider keeping a nil guard:
if buckets == nil || buckets.Error != nil {
if buckets != nil {
return nil, buckets.Error
}
return nil, nil
}Or verify that GetBuckets() is guaranteed to never return a nil result struct.
| @@ -1028,6 +1028,9 @@ func (g *mqlGcpProjectGkeServiceClusterNetworkConfig) network() (*mqlGcpProjectC | |||
|
|
|||
| // Format is projects/project-1/global/networks/net-1 | |||
| params := strings.Split(networkPath, "/") | |||
There was a problem hiding this comment.
🔵 suggestion — The bounds check len(params) < 2 only protects params[1], but params[len(params)-1] a few lines above would also need len(params) >= 1. More importantly, the path format is projects/project-1/global/networks/net-1 (5 segments), so a stricter check like len(params) < 5 would better validate the expected structure, consistent with the checks added in common.go.
1 participant
Summary
discovery.go(inverted nil check on buckets)log.Fatalcalls with error returns indiscovery.goListRepositorycloudrun.go(break on error instead of falling through)cloudrun.go,redis.go,filestore.go,kms.goinstance.SettingsandBackupRetentionSettingsinsql.goCreateResource/cryptoKeyVersionToMqlinkms.goCertChainsin KMS attestation handlingpolicy.InternalProtoinpubsub.goIAM policy methodscommon.goandgke.goPlatformIdsincommon.gogetAssetIdentifierdataproc.goVirtualClusterConfig and nodePoolTargetToMqlTest plan
go vet ./resources/passes (verified locally)go build ./...passes (verified locally)mql shell gcpand verify basic resource queries still work🤖 Generated with Claude Code